Gml 1809 feedback analysis access control #256
Conversation
luzhoutg
commented
Jul 31, 2024
•
luzhoutg
commented
- update feedback analytics notebook with get_feedback endpoint
- add access control
- add test cases
- update the loadConfig to load multiple config files
luzhoutg
requested review from
billshitg,
parkererickson-tg and
RobRossmiller-TG
chat-history/routes/routes.go
Outdated
| if tgcloud { | ||
| requestURL = fmt.Sprintf("%s:443/gsqlserver/gsql/file", host) | ||
| } else { | ||
| requestURL = fmt.Sprintf("%s:14240/gsqlserver/gsql/file", host) |
There was a problem hiding this comment.
For on-prem DBs, the port is not always 14240. Could we read it from config? How are we handling it for copilot?
There was a problem hiding this comment.
Thanks for pointing this out. Just push the updated. For CoPilot, pyTG have handled it already.
chat-history/config/config.go
Outdated
| b, err = os.ReadFile(path) | ||
| if err != nil { | ||
|
|
||
| if err := json.Unmarshal(b, &cfg); err != nil { |
There was a problem hiding this comment.
If you're reading multiple config files, won't cfg just be the last file that was parsed in the list?
There was a problem hiding this comment.
Good catch. I update the loadConfig to load multiple config file in to different config structs. i.e., ChatDbConfig, and TgDbConfig. It looks like this: router.HandleFunc("GET /get_feedback", routes.GetFeedback(cfg.TgDbConfig.Hostname, cfg.TgDbConfig.GsPort, cfg.ChatDbConfig.ConversationAccessRoles, cfg.TgDbConfig.TgCloud))
There was a problem hiding this comment.
also added test cases for non-admin users and non-existent users.
